Add RAII for PIDL types (BSTR-style) and FreeWith(CoTaskMemFree) for CoTaskMem string out-params#2276
Draft
antmor wants to merge 3 commits into
Draft
Add RAII for PIDL types (BSTR-style) and FreeWith(CoTaskMemFree) for CoTaskMem string out-params#2276antmor wants to merge 3 commits into
antmor wants to merge 3 commits into
Conversation
…feHandle Model PIDLIST_RELATIVE, PIDLIST_ABSOLUTE, and PITEMID_CHILD as NativeTypedef handles with CloseApi=ILFree in autoTypes.json, following the HLOCAL/HGLOBAL (void* + free-function) precedent. This lets CsWin32 emit an ILFree-based SafeHandle for PIDLs instead of requiring consumers to hand-roll RAII around SHGetIDListFromObject, ILCreateFromPath, SHGetKnownFolderIDList, etc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…or string out-params - PIDL typedefs (PIDLIST_RELATIVE/ABSOLUTE, PITEMID_CHILD) now follow the BSTR pattern: NativeTypedef wrapping the real ITEMIDLIST* pointee (no void*) with CloseApi=CoTaskMemFree, so CsWin32 generates a strongly-typed SafeHandle. Namespace is inherited from the CloseApi (as BSTR/HANDLE do). - Add FreeWith(CoTaskMemFree) annotations for PWSTR/LPOLESTR out-params that must be freed with CoTaskMemFree (the unique_cotaskmem_string cases): SHGetNameFromIDList, StringFromCLSID, StringFromIID, IShellItem::GetDisplayName. Mirrors the existing SHGetKnownFolderPath::ppszPath precedent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Placing the PIDL NativeTypedefs in the CloseApi's inherited namespace put them in System.Com while their ITEMIDLIST* payload lives in Shell.Common, creating a System.Com <-> Shell.Common cycle (NoCyclicalNamespaces failure). Fix keeps the strong ITEMIDLIST* typing by co-locating everything in Shell.Common: - PIDL typedefs (PIDLIST_RELATIVE/ABSOLUTE, PITEMID_CHILD) explicitly in Windows.Win32.UI.Shell.Common with ValueType=ITEMIDLIST* and CloseApi=ILFree. - Move ILFree into Shell.Common via requiredNamespacesForNames.rsp so the CloseApi is intra-namespace (satisfies the same-namespace rule, adds no outward edge). - Restore InvalidHandleValues=[0] (required by RaiiFreeAttributeTests). Validated locally against MSVC 14.44: all tests pass (MetadataUtils 17/17, Win32MetadataScraper 44/44, Windows.Win32.Tests 13/13). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
Thanks for the contribution @antmor! I see this is still a draft, are you still working on this? Did you want feedback on it? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related improvements so CsWin32 consumers get RAII/free guidance for shell PIDLs and CoTaskMem-allocated strings instead of hand-rolling cleanup.
1. PIDL RAII (autoTypes.json) - BSTR-style typedef
Model
PIDLIST_RELATIVE,PIDLIST_ABSOLUTE, andPITEMID_CHILDasNativeTypedefhandles following theBSTRprecedent (a distinct typedef wrapping its real pointee,char*, with aCloseApi):{ "Name": "PIDLIST_ABSOLUTE", "ValueType": "Windows.Win32.UI.Shell.Common.ITEMIDLIST*", "CloseApi": "CoTaskMemFree", "AlsoUsableFor": "PIDLIST_RELATIVE", "NativeTypedef": true }ITEMIDLIST*pointee (novoid*), analogous to BSTR'schar*.CloseApi = CoTaskMemFree-> CsWin32 generates a SafeHandle.AlsoUsableForlinks absolute/child to relative (asHMODULE/HINSTANCEdo).BSTR/HANDLEdo).2. FreeWith(CoTaskMemFree) for CoTaskMem string out-params (emitter.settings.rsp)
The
unique_cotaskmem_stringcases -PWSTR/LPOLESTRout-params that must be freed withCoTaskMemFree, mirroring the existingSHGetKnownFolderPath::ppszPathprecedent:SHGetNameFromIDList::ppszNameStringFromCLSID::lplpszStringFromIID::lplpszIShellItem::GetDisplayName::ppszNameOpen questions / please validate in CI
ValueType(ITEMIDLIST*) has no existing precedent (all current entries use primitives/void*/IntPtr). Confirm the generator/emitter accepts a fully-qualified struct-pointer payload.CoTaskMemFreenamespace: if it resolves to multiple namespaces the same-namespace check throws; also inheritingSystem.Comrelocates the PIDL typedefs fromUI.Shell.Common- the winmd baseline diff will show this. If undesirable, alternatives are an explicitNamespaceor usingILFree(fewer partitions).Testing
Local
DoAll.ps1scrape is blocked in my environment by a VS 2026 / MSVC 14.51 vs pinned libclang 17 mismatch (__builtin_verbose_trap), unrelated to this change. Relying on CI (pr-validation) for the winmd/API diff.